HADOOP-19617. [JDK17] Remove JUnit4 Dependency.#7957
Merged
slfan1989 merged 3 commits intoapache:trunkfrom Sep 15, 2025
Merged
Conversation
github-actions
Bot
added
YARN
MapReduce
HDFS
build
trunk
Common
TOOLS
ABFS
DYNAMOMETER
labels
slfan1989
changed the title
Contributor
Author
|
@cnauroth @ayushtkn @steveloughran @Hexiaoqiao @anujmodi2021 @ahmarsuhail @szetszwo @jojochuang This is our final PR related to the JUnit 5 upgrade. Once this is merged, the upgrade on the trunk branch will be complete. Could you help review this PR ? Thank you very much! cc: @zhtttylz |
|
💔 -1 overall
This message was automatically generated. |
slfan1989
force-pushed
the
HADOOP-19617-All
branch
from
4878f2b to
9665314
Compare
|
💔 -1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
anujmodi2021
approved these changes
Sep 15, 2025
|
💔 -1 overall
This message was automatically generated. |
cnauroth
approved these changes
Sep 15, 2025
Contributor
cnauroth
left a comment
cnauroth
left a comment
There was a problem hiding this comment.
+1. Thank you @slfan1989 !
Contributor
Author
|
The build result of this PR is as expected, and we are now able to remove the dependency on JUnit 4. There is only one remaining test case with an issue, which we plan to address in a follow-up PR. Many thanks for everyone's support! With this change, the Hadoop trunk branch has completely removed the dependency on JUnit 4 and has fully migrated to JUnit 5. |
Contributor
Author
|
Many thanks to @anujmodi2021 and @cnauroth for reviewing this PR! |
Congratulations for getting this huge overhaul through! |
Contributor
|
@slfan1989 and @zhtttylz , thank you so much for all of your efforts on the JUnit5 patches! |
Contributor
|
That's a great contribution. Thanks everyone! |
Contributor
|
BTW, filed HADOOP-19692 for excluding junit 4 transitive dependency. |
Contributor
Author
|
@cnauroth @h-vetinari @szetszwo @zhtttylz @anujmodi2021 Sorry for the late reply, and thank you again for your help! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of PR
HADOOP-19617 - [JDK17] Remove JUnit4 Dependency.
This is our final PR for the
JUnit 5upgrade. In this PR, we are completely removing all relatedJUnit4 dependencies. As a result,org.hamcrest, which was previously brought in viaJUnit 4, will no longer be available. This leads to a few minor code changes across three modules. Given the small scope of these changes, I’ve decided to include them in this PR instead of creating multiple separate ones.The details are as follows:
The original code made use of
org.hamcrest. I have replaced it with AssertJ assertions to align withJUnit 5standards.This class is currently unused in the project, so I’ve removed it.
The original use of JUnit 4’s
@RunWith(Parameterized.class)has been migrated to JUnit 5’s@ParameterizedClassto support parameterized testing.Similarly, since this class is not being referenced anywhere, I’ve removed it as well.
How was this patch tested?
CI Test.
For code changes:
LICENSE,LICENSE-binary,NOTICE-binaryfiles?